Skip to content

fix: engine exec safety, safewrite adoption, flock session lock, docs truth - #201

Merged
Patel230 merged 14 commits into
mainfrom
fix/audit-sweep-2026-08
Aug 16, 2026
Merged

fix: engine exec safety, safewrite adoption, flock session lock, docs truth#201
Patel230 merged 14 commits into
mainfrom
fix/audit-sweep-2026-08

Conversation

@Patel230

@Patel230 Patel230 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Deep code-quality audit fixes for the main hawk repo:

  • Engine subprocess safety — all engine exec paths now bounded by named timeouts (auto-commit 2m, validate 5m, experiment-loop rollback 2m); errors surfaced via slog instead of swallowed
  • Shell injection surface removed — deleted dead VerifyCommandSucceeds (zero callers, ran sh -c bypassing safety stack); RunScript no longer double-evals through sh -c
  • safewrite adoption — 5 state-write sites (global settings, session checkpoints, handover) now use atomic tmp+rename+fsync+symlink-refusal via internal/safewrite
  • Session lock → flock — replaced TOCTOU-racy stat/remove/O_EXCL lockfile with gofrs/flock; PID file kept for diagnostics only
  • Error visibility — memory persistence failures in engine/stream logged; self_improve persistence chain returns errors; config-panel provider write failure surfaced to user
  • Lint pin — Makefile golangci-lint version now matches CI (v2.1.0)
  • Docs truth — SECURITY/CONTRIBUTING stripped of polyglot template; distinctive targets documented; plan docs moved to docs/plans/
  • Golden help — regenerated for trace's entiretrace rename

✅ Submodule pins updated — ready for merge

All 8 dependency PRs are merged, and the submodule pointers have been bumped to the merged commits:

Submodule Pinned commit Source
eyrie ede6671 main (PR #110 merged)
hawk-core-contracts 16ebcfd main (PR #27 merged)
hawk-mcpkit 4a5ea25 main (PR #9 merged)
inspect b5b6584 fix-branch tip (includes re-pin to merged contracts)
sight 481d3d1 fix-branch tip (includes re-pin to merged contracts)
tok 643b667 main (PR #81 merged)
trace 59b437b main (PR #66 merged)
yaad 42bdda9 main (PR #60 merged)

go.mod requires updated to matching pseudo-versions; check-submodule-release-parity.sh passes for all 8 modules.

Test plan

  • go build ./... clean against the submodule fix branches
  • make boundaries — all guards pass
  • Full suites: internal/engine, internal/session (incl. -race on new lock tests), internal/config, bridges, executiongraph
  • Pre-push: full go test ./... (130+ packages, 0 failures) + boundary guards + govulncheck + vet

Experiment-loop rollback, auto-commit git calls, and the post-edit syntax
validators all ran on context.Background() with ignored errors, so a hung
git/go vet/npx invocation could block a session forever and rollback
failures were invisible.

- experiment loop: thread the request ctx into snapshot(); rollback uses
  a detached, time-bounded ctx (gitRollbackTimeout) because a revert must
  still complete after cancellation; log failed rollbacks and failed
  HEAD lookups instead of discarding the errors
- auto-commit: bound every git call with autoCommitTimeout; log git
  status failures instead of treating them as 'no changes'
- validators: bound go vet / python3 / node / npx tsc with
  validatorTimeout so a wedged toolchain cannot hang post-edit checks

No commands, commit messages, or validation logic changed.
…pt eval

- AssumptionTracker.VerifyCommandSucceeds ran caller-supplied strings
  through `sh -c`, bypassing the permission/safety stack. It had zero
  callers (verified repo-wide) — delete it rather than keep the surface.
- SelfHealer.RunScript executed the script path through `sh -c <path>`,
  re-parsing the path as shell code (double evaluation). Invoke the path
  directly via /bin/sh; timeout, capture, and exit-code handling are
  unchanged and shebang-led scripts behave identically.
Global settings, checkpoint file contents, checkpoint restores, handovers,
and named checkpoints went through plain os.WriteFile (or a hand-rolled
tmp+rename), leaving them open to partial writes and symlink substitution
at the destination.

All five sites already wrote mode 0600, which is exactly what
safewrite.WriteFile produces, so file modes are unchanged while every
write becomes atomic (fsync + rename) and symlink-resistant:

- internal/config/settings.go SaveGlobal
- internal/session/checkpoint.go saveFileContents / restoreFileContents
  (restore now refuses to write through a symlinked destination and
  fails loudly instead)
- internal/session/handover.go SaveHandover
- internal/session/named_checkpoint.go SaveNamedCheckpoint
AcquireLock used stat → stale-if->5min → remove → O_EXCL create, so a
live lock could be deleted whenever staleness was misjudged (slow holder,
clock skew), letting two instances open the same session. Mutual
exclusion now comes from an OS advisory lock (gofrs/flock, promoted to a
direct dependency); a crashed holder's lock is reclaimed instantly
because the kernel drops the flock at process death — no stale window at
all. The lock file keeps PID + timestamps purely as diagnostics
(lockStaleAfter now only logs a hung-holder hint on contention), and
Release keeps the file with a released marker to avoid the
unlock-then-unlink split-brain race.

- gofrs/flock v0.13.0 promoted from indirect to direct require;
  go.work untouched
- exported API shape preserved (AcquireLock/Release/Refresh/
  SessionLockedError); existing lock tests unchanged and passing
- new test: 8 concurrent acquirers, exactly one holder; new test for
  instant crash reclaim
Memory persistence failures were silently discarded in four stream-loop
sites (assistant learnings, skills, conversation summaries, insights);
they now log via slog like the existing background-remember path. The
self-improve lesson store ignored the whole persist chain (mkdir /
marshal / write) and silently swallowed corrupt loads — save now returns
an error that Learn/Clear log, and a corrupt store logs and starts
empty. The config panel dropped three SetGlobalSetting(provider)
failures on the floor; the model selection flow now reports the failure
through the panel's error-message mechanism instead. Provider
precedence (manual pick > gateway > provider) is unchanged.
The lint, lint-fix, and setup targets installed golangci-lint@latest while
CI pins v2.1.0, so local lint results could diverge from the gate. Pin the
Makefile to the same version via a GOLANGCI_VERSION variable; the install
mechanism (go install on first miss) is unchanged.
SECURITY.md and CONTRIBUTING.md still carried the polyglot template's
ruff / mypy / pip-audit / pnpm-lock / pyproject.toml language; this is a
pure-Go repo. They now describe the real tools (golangci-lint with gosec
rules, go vet, govulncheck) and CONTRIBUTING gains the repo's own dev
targets (make setup / boundaries / test-10x / smoke).

SPEC_DRIVEN_PLAN.md, SPEC_DRIVEN_PHASE2_PLAN.md, and
internal/engine/REFACTOR_PLAN.md move into docs/plans/ (the refactor plan
as engine-refactor-plan.md); all code references to the old locations
are updated.
external/trace rebranded its root command from 'entire' to 'trace'
(branch fix/audit-sweep-2026-08); the golden help snapshot now reflects
the mounted command list.
@Patel230
Patel230 marked this pull request as draft August 16, 2026 03:15
Point all 8 ecosystem submodules to their merged main after the
fix/audit-sweep-2026-08 merges:
  - hawk-core-contracts: strict parsers, FailOn fix, Finding validation
  - yaad: cascade deletes, DSN pragmas, chunking, backup fsync, key policy
  - hawk-mcpkit: SSE body cap parity, dead code removal, tool-search docs
  - inspect: FailOn contract wiring, findings store retry/drop
  - sight: FailOn contract wiring, dead graph/audit removal
  - eyrie: Concentrate timeout/retries, stream diagnostics, bodyclose
  - tok: estimator cache bounds, RestorationTracker cap
  - trace: entire→trace rebrand, hook binary resolution, settings migration
@Patel230
Patel230 marked this pull request as ready for review August 16, 2026 04:20
- Update to ede667174 (main branch after PR #110 merge)
All 8 dependency PRs are merged. Per the merge order in the PR
description, bump every submodule pointer to the merged commit:

- eyrie, hawk-core-contracts, hawk-mcpkit, tok, trace, yaad: merged main
- inspect, sight: fix-branch tips (include the required
  're-pin hawk-core-contracts to merged main' commits for
  standalone module builds)

go.mod requires updated to the matching pseudo-versions.
@Patel230
Patel230 merged commit f97a0eb into main Aug 16, 2026
26 checks passed
@Patel230
Patel230 deleted the fix/audit-sweep-2026-08 branch August 16, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant